Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce zephyr into firmware monorepo #4449

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

TychoVrahe
Copy link
Contributor

@TychoVrahe TychoVrahe commented Dec 16, 2024

This PR introduces zephyr, or rather NCS, to firmware monorepo. Zephyr is used for implementing a BLE gateway.

There is still work to be done, but i would like to get this into main soon, as it will allow more incremental work and ease review of things that affect both core and BLE, like communication and so.

But this should be good enough to:

  • run on both T3W1 boards, the devkit1 and revA
  • should be clear from readme itself how to build and flash this.

Reviewers:
@matejcik please check the structure of the added stuff
@cepetr general architectural comments, no need to deep dive into the code, unless you want to.
@hiviah please look at the BLE specific code in /nordic/trezor/trezor-ble

fixes #4301

@TychoVrahe TychoVrahe self-assigned this Dec 16, 2024
Copy link

github-actions bot commented Dec 16, 2024

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@TychoVrahe TychoVrahe force-pushed the tychovrahe/zephyr/introduction branch from bc3bbe8 to 09a894e Compare December 16, 2024 10:22
@TychoVrahe TychoVrahe force-pushed the tychovrahe/zephyr/introduction branch 4 times, most recently from 10ed2af to a260f74 Compare January 20, 2025 16:03
@TychoVrahe TychoVrahe force-pushed the tychovrahe/zephyr/introduction branch 2 times, most recently from cae09d4 to cc950f7 Compare January 23, 2025 09:19
@TychoVrahe TychoVrahe marked this pull request as ready for review January 23, 2025 09:41
@TychoVrahe TychoVrahe force-pushed the tychovrahe/zephyr/introduction branch from cc950f7 to 58b321a Compare January 24, 2025 08:42
@hiviah
Copy link
Contributor

hiviah commented Feb 4, 2025

I'll add notes as they come to me.

We seem to have mcuboot overlay missing in nordic/trezor/west.yml, here is a sample one, we need our own fork of mcuboot.

    - name: mcuboot
      url: https://github.com/hiviah/mcuboot
      path: bootloader/mcuboot
      revision: edfe1e1465dbc698bf9a195816247913490ab391

Also mcboot itself has several boots inside it, IIRC we are using the boot_serial one (need to keep in mind when one changes things and things don't appear to change).


Update the modules:
```sh
west update
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't yet find a way to have common SDK source so that it's not repeated by each repo clone (since now whole repo build is doubled to some 8 GB). Theoretically only build dir needs to be separate per project, but by default they clobber over each other.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we probably could put the west.yml into firmware repo root. then it would initialize the SDK outside of the firmware repo directory, and you could share it, but potential differences between west.yml might cause headaches.

But you get the additional SDK bloat only per clone when you actually west init and west update, which might not be that often

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have lots of clones usually, 12 ATM and that is after I cleaned up some. I keep some older milestones, because due to number of components it's not always easy or possible to go back if you need to try if a bug was introduced by one of 3 compiler(s), components.

What I meant I think from the docs you can have one SDK dir - and inside it several versions of SDK like 2.6.2 or 2.7.0, and your build is outside the SDK dir structure and is always separate.

Once you run the insane ncs shell you set the build for a particular version of SDK+trezor-ble, but no need to have extra SDK clones.

At least in this case west seems to helpful by giving you hint to use --build-dir to avoid SDK clones by:

ERROR: Build directory "/home/ondro/work/satoshilabs/repos/ncs/build" is for application "/home/ondro/work/satoshilabs/repos/ncs/testapp3", but source directory "/home/ondro/work/satoshilabs/repos/trezor-firmware.new-zephyr/nordic/trezor" was specified; please clean it, use --pristine, or use --build-dir to set another build directory

So for example you can make even builds for different boards with the same SDK just by changing the build dir (here e.g. run from ncs shell innordic directory):

west build trezor/./trezor-ble -b t3w1_d1_nrf52833 --build-dir=bleeeee --sysbuild

Unfortunately it seems build needs to be working directory with all its shitty mess modules brought in, spawn of Santa (you'd wish it was Satan) with all-year christmas music playing non-stop for eternity in closed room.

Copy link
Contributor

@hiviah hiviah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not tested on device, but otherwise LGTM.

Just make the nordic/trezor/west.yml to point to our mcuboot that needs to be created and check it builds (took the rev id from what we are using to build now with 2.6.2) :

- name: mcuboot
  url: https://github.com/trezor/mcuboot
  path: bootloader/mcuboot
  revision: daf2946a0f07a14b57bd69d29ac4cdde6f810fb7

@@ -11,3 +11,13 @@ __pycache__/
proto.gv*
.DS_Store
crypto/tests/libtrezor-crypto.so.dSYM/
/nordic/.west/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about to move these into /nordic/.gitignore folder?

@TychoVrahe
Copy link
Contributor Author

We seem to have mcuboot overlay missing in nordic/trezor/west.yml, here is a sample one, we need our own fork of mcuboot.

Yes, lets do it in separate PR, as we don't have proper repo yet. I created an issue: #4585

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏃‍♀️ In progress
Development

Successfully merging this pull request may close these issues.

Advertise colour together with name of the device over BT
4 participants